fix: actionable slug-conflict error when publishing a staged slug#2036
Open
swissky wants to merge 2 commits into
Open
fix: actionable slug-conflict error when publishing a staged slug#2036swissky wants to merge 2 commits into
swissky wants to merge 2 commits into
Conversation
A slug edit staged as `_slug` in a draft revision lands on the live slug column only inside repo.publish(), bypassing the uniqueness handling that guards direct slug edits. When the staged slug collided with another entry's (slug, locale), the UNIQUE constraint fired mid-publish and surfaced as an opaque 500 with a raw D1/SQLite message — after draft data had already been synced into the live columns. publish() now validates the staged slug before any write (against all rows, including drafts and trashed entries, since the unique index covers them) and throws an EmDashValidationError naming the conflicting slug and entry. handleContentPublish additionally maps a unique-constraint fingerprint to SLUG_CONFLICT as a backstop for the race window, matching create/update. Fixes emdash-cms#2034
Adversarial-review follow-ups: tag the pre-check error so publish maps it to the same 409 SLUG_CONFLICT as create/update (not a generic 400), write the slug before syncing data columns so a constraint hit in the race window leaves no partial publish on D1, and skip the check for NULL locales (unique indexes treat NULLs as distinct).
🦋 Changeset detectedLatest commit: 5a53e1c The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
area/core
size/M
review/needs-review
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
swissky
added
bot:review
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
This is the right bug fix for the right problem, and the implementation fits EmDash’s architecture.
Approach judgment
Publishing a draft revision is the only path where _slug lands on the live slug column, so it was the only slug mutation that bypassed the existing create/update conflict-mapping path. Reordering the publish writes so the slug UPDATE happens before syncDataColumns is the correct D1-specific mitigation: D1 does not support multi-statement transactions (transaction.ts falls back to running the callback directly), so isolating the unique-constraint statement first prevents a partial publish if a concurrent writer takes the slug. Adding a pre-check inside publish() and a backstop message-fingerprint mapping in handleContentPublish mirrors the existing create/update handling exactly.
What I checked
- Read the diff and the full changed files.
- Verified the unique constraint is
UNIQUE(slug, locale)on content tables (migration019_i18n.tsandschema/registry.ts), so the newfindBySlugIncludingTrashedcall withexisting.localecovers the same row set the index would reject. - Confirmed the new query filters by
localeand includes trashed rows, matching the PR’s stated behavior. - Checked SQL safety: no raw interpolation; identifiers use
sql.ref(), values are parameterized. - Checked API envelope shape: both the new
EmDashValidationErrorbranch and the backstop return{ success: false, error: { code, message } }. - Checked error-code mapping: the
details.code === "SLUG_CONFLICT"path returns the sameSLUG_CONFLICTcode as create/update, and the fallback fingerprint uses the sameunique constraint failed/duplicate key+slugheuristic. - Reviewed the changeset: present, targeted, and written for a user upgrading the package.
- Reviewed tests: they cover conflict with a published entry, conflict with a trashed entry, republishing with the own slug, cross-locale reuse, and the handler-level
SLUG_CONFLICTmapping.
Conclusion
The change is minimal, correctly scoped, leaves the live row untouched on conflict, and has good regression tests. No AGENTS.md conventions are violated and I found no logic bugs. LGTM.
github-actions
Bot
added
review/approved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Publishing an entry whose staged slug (stored as
_slugin a draft revision) collides with another entry's(slug, locale)failed with an opaque 500 and a raw D1/SQLite message (D1_ERROR: UNIQUE constraint failed: ec_news.slug, ec_news.locale) — after draft data had already been synced into the live columns. The staged slug lands on the live column only insiderepo.publish(), bypassing the uniqueness handling that guards direct slug edits.This PR:
publish()before any write, against all rows in the entry's locale — including drafts and trashed entries, since the(slug, locale)unique index covers them. On conflict it throws a validation error naming the slug and the conflicting entry id, which the handler maps to the sameSLUG_CONFLICT(409) as direct slug edits in create/update.UPDATE(the only statement that can hit the constraint) runs beforesyncDataColumns— if a concurrent write takes the slug in the race window, no partial publish is left behind on D1 (which has no transactions to roll back).handleContentPublish's catch toSLUG_CONFLICTas a backstop for that race window, matching the existing create/update handling — a raw SQLite error never reaches the client.NULLlocale (unique indexes treat NULLs as distinct, so those rows can't collide).Closes #2034
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain.AI-generated code disclosure
Screenshots / test output
Regression tests: staged slug conflicting with a published entry, with a trashed entry, republish with own slug, same slug across locales, and the handler-level
SLUG_CONFLICTmapping.